Skip to content

Conversation

@koesie10
Copy link
Member

This fixes npm run build-storybook by ensuring we're not importing anything from the path module in stories.

@koesie10 koesie10 marked this pull request as ready for review February 19, 2025 09:20
Copilot AI review requested due to automatic review settings February 19, 2025 09:20
@koesie10 koesie10 requested review from a team as code owners February 19, 2025 09:20
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

PR Overview

This pull request fixes the Storybook build by removing any reliance on the Node "path" module in stories. The change removes the import from "path" and replaces the usage of path.join with a template literal for constructing the yamlPath.

Changes

File Description
extensions/ql-vscode/test/factories/model-editor/extension-pack.ts Removed Node "path" dependency to support Storybook build

Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.

Comments suppressed due to low confidence (1)

extensions/ql-vscode/test/factories/model-editor/extension-pack.ts:9

  • While the template literal approach avoids importing 'path', it may not handle platform-specific file separators (e.g., Windows '\' vs UNIX '/'). If cross-platform compatibility is required, consider an alternative solution that preserves this behavior without importing 'path'.
yamlPath: `${path}/codeql-pack.yml`,

Tip: If you use Visual Studio Code, you can request a review from Copilot before you push from the "Source Control" tab. Learn more

Copy link
Contributor

@robertbrignull robertbrignull left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

Should build-storybook be added to .github/workflows/main.yml with the other build/linting CI jobs so we don't break it again in future?

@koesie10
Copy link
Member Author

Should build-storybook be added to .github/workflows/main.yml with the other build/linting CI jobs so we don't break it again in future?

That's a good idea. I can definitely add that, we can even publish the stories to GitHub Pages. I'll do that in a follow-up PR though since it's unrelated to this.

@koesie10 koesie10 merged commit 24deccf into main Feb 19, 2025
37 of 39 checks passed
@koesie10 koesie10 deleted the koesie10/fix-storybook branch February 19, 2025 10:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants